Repository navigation
Conversation
…func() A single-phase init function that creates its module using only the last component of a dotted name (as pybind11 does for submodules) got a module whose __name__ was the short name. Set the package context while calling the init function, as the dynamic loader does, so the module gets the full name from the spec. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HoZE3WdRWQshvinfBZ137N
…nitfunc() The builtin loader info encoded the module name as ASCII, so a non-ASCII spec name failed with UnicodeEncodeError even for multi-phase init modules, which support such names when loaded dynamically. Fall back to UTF-8 and mark the name as non-ASCII, so that multi-phase init works and single-phase init is rejected with the same error as for dynamically loaded extensions.
This private function was added in 291cfa4 but never called.
It is always set to info->name.
PyUnicode_AsUTF8 might malloc.
encukou
changed the title
_Py_ext_module_loader_info initialization_Py_ext_module_loader_info initialization
Documentation build overview
|
itamaro
left a comment
itamaro
left a comment
There was a problem hiding this comment.
unification and refactor looks good to me!
requesting changes for the bug (inline comment), and also flagging that there's a user-visible behavior change with dotted single-phase init modules in inittab -- this changes their __name__ to be the full dotted name. I think this is a good change, but since it's potentially a breaking change, it should probably be called out in the 3.16 release notes.
| info.filename = PyObject_GetAttr(spec, &_Py_ID(origin)); | ||
| if (info.filename == NULL) { | ||
| Py_DECREF(name); | ||
| return -1; |
There was a problem hiding this comment.
I think this is a bug.
when the spec has no origin attribute, this would null-deref
repro (that you can turn into a regression test)
class Spec:
name = "abc"
_imp.create_dynamic(Spec())
on main, this raises AttributeError, on this PR it segfaults. works with the suggested fix:
| info.filename = PyObject_GetAttr(spec, &_Py_ID(origin)); | |
| if (info.filename == NULL) { | |
| Py_DECREF(name); | |
| return -1; | |
| info.filename = PyObject_GetAttr(spec, &_Py_ID(origin)); | |
| if (info.filename == NULL) { | |
| goto error; |
|
When you're done making the requested changes, leave the comment: |
Well, that's for #159316, which is now in 3.15 as well... |
| modname = _PyObject_CallMethod(encoded, &_Py_ID(replace), "cc", '-', '_'); | ||
| if (modname == NULL) | ||
| goto error; | ||
| /* Call encoded.replace('-', '_') */ |
There was a problem hiding this comment.
why is this normalization happening only in the non-ASCII branch now?
iiuc, it should remain shared for both ASCII and non-ASCII.
example that worked before and would fail after:
import _heapq
import importlib.util
spec = importlib.util.spec_from_file_location("-heapq", _heapq.__file__)
module = importlib.util.module_from_spec(spec)
| if (initfunc != NULL) { | ||
| /* An explicitly provided init function (see | ||
| * PyImport_CreateModuleFromInitfunc()) may belong to a submodule | ||
| * whose single-phase init creates the module using only the last | ||
| * component of the name (as e.g. pybind11 does). Set the package | ||
| * context so that PyModule_Create() resolves the full name, the | ||
| * same as for dynamically loaded extensions. */ | ||
| info.newcontext = PyUnicode_AsUTF8(info.name); | ||
| if (info.newcontext == NULL) { | ||
| _Py_ext_module_loader_info_clear(&info); | ||
| return NULL; | ||
| } | ||
| } |
There was a problem hiding this comment.
is this entire block now redundant? (because _Py_ext_module_loader_info_init would already set info.newcontext unconditionally)
2 participants
This builds on #157387 and #157390 with a larger refactor.
#157387 should be merged first, but I'd like to get this one reviewed before that, in case we find something else to backport.
_Py_ext_module_loader_info_init*functions. The “core” one was entirely unused, and the difference between “builtin” and “extension” can be expressed in arguments.imp.create_builtinwith empty name.)Core modules (
sysandbuiltins) still don't go through the routine.PyImport_CreateModuleFromInitfunc(): wrong__name__for submodules, non-ASCII names rejected, inittab name clashes #157384